Skip to content

fix: Refuse to use private keys that are group or world readable#4879

Open
johnsaigle wants to merge 4 commits into
wormhole-foundation:mainfrom
johnsaigle:fix-readable-keys
Open

fix: Refuse to use private keys that are group or world readable#4879
johnsaigle wants to merge 4 commits into
wormhole-foundation:mainfrom
johnsaigle:fix-readable-keys

Conversation

@johnsaigle

@johnsaigle johnsaigle commented Jun 19, 2026

Copy link
Copy Markdown
Contributor
  • Added common.ValidatePrivateKeyFilePermissions.
  • LoadArmoredKey now rejects regular private-key files with any group/other permissions: mode.Perm() & 0077 != 0.
  • NewFileSigner now reuses common.LoadArmoredKey, removing duplicated key decoding.
  • LoadWormchainPrivKey now validates permissions before reading.
  • Added tests for unsafe 0644 key files in:
    • node/pkg/common/armoredKey_test.go
    • node/pkg/guardiansigner/guardiansigner_test.go
    • node/pkg/wormconn/wormchain_key_test.go
  • Updated query tests to copy the checked-in dev key into a temp 0600 file before loading.

@johnsaigle
johnsaigle marked this pull request as ready for review July 16, 2026 12:32
- Added common.ValidatePrivateKeyFilePermissions.
- LoadArmoredKey now rejects regular private-key files with any group/other permissions: mode.Perm() & 0077 != 0.
- NewFileSigner now reuses common.LoadArmoredKey, removing duplicated key decoding.
- LoadWormchainPrivKey now validates permissions before reading.
- Added tests for unsafe 0644 key files in:
- node/pkg/common/armoredKey_test.go
- node/pkg/guardiansigner/guardiansigner_test.go
- node/pkg/wormconn/wormchain_key_test.go
- Updated query tests to copy the checked-in dev key into a temp 0600 file before loading.
- Add unit test to maintain code coverage that would've dropped due to
  refactoring
bemic
bemic previously approved these changes Jul 16, 2026

@bemic bemic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@johnsaigle

Copy link
Copy Markdown
Contributor Author

@bemic please re-approve, I had to update the permissions on the CCQ key (see latest commit)

@johnsaigle
johnsaigle requested a review from bemic July 16, 2026 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants